test: derive max binary length dynamically from Notecard#233
Closed
zakoverflow wants to merge 1 commit intoblues:masterfrom
Closed
test: derive max binary length dynamically from Notecard#233zakoverflow wants to merge 1 commit intoblues:masterfrom
zakoverflow wants to merge 1 commit intoblues:masterfrom
Conversation
Remove the hardcoded per-target constants (R5_MAX_BINARY_LENGTH=130554, U5_MAX_BINARY_LENGTH=261110) and the get_expected_max_binary_length() helper that compared them against card.binary "max". The assertion against a hardcoded value caused test_get_max_binary_length to fail whenever the nightly Notecard firmware changed the reported max (e.g. 261110 -> 261114 on 2026-02-02), even though the SDK and the Notecard were both functioning correctly. The Notecard firmware is the authoritative source of truth for the maximum binary buffer size. Tests that follow test_get_max_binary_length already use the runtime-queried max_binary_length value; the equality assertion added no regression protection beyond what those downstream tests already provide. The minimum sanity check (max >= 1024) is retained.
Contributor
Author
|
Closing — the workflow_dispatch trigger already exists. Opening this PR was a misread of the request. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The
test_get_max_binary_lengthHIL test was asserting that the Notecard's reportedcard.binarymax matched a hardcoded per-target constant (R5_MAX_BINARY_LENGTH=130554,U5_MAX_BINARY_LENGTH=261110). This caused the test to fail whenever a nightly Notecard firmware update changed the reported max (e.g. 261110 → 261114 on 2026-02-02), even though the SDK and Notecard were both functioning correctly.As a result, the
workflow_dispatchtrigger (on-demand run) was effectively broken — triggering the tests manually would fail not because of any SDK bug, but because of a stale expected value.Fix
Remove the hardcoded constants and the
get_expected_max_binary_length()helper. The Notecard firmware is the authoritative source of truth for the maximum binary buffer size.The downstream tests that follow
test_get_max_binary_lengthalready use the runtime-queriedmax_binary_lengthvalue, so no regression coverage is lost. The minimum sanity check (max >= 1024) is retained.Impact
notecard-binary-testsworkflow can now be triggered on demand (workflow_dispatch) and run against any Notecard firmware without pre-updating hardcoded constants.note-clibrary code — test-only change.